Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[AMD] Always swap operands of mfma and use mfma.transposed layout #4767

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

zhanglx13
Copy link
Collaborator

@zhanglx13 zhanglx13 commented Sep 20, 2024

  • Fixed the issue with getOrder for mfma layout
  • Fixed the issue with reduceOp when dealing with mfma.transposed layout

Also fixed the issue with getOrder for mfma layout
In general, we should use getThreadOrder in most places where getOrder
is called. Note that order and threadOrder can be different, and this
is the case for mfma.transposed layout.
Copy link
Collaborator

@antiagainst antiagainst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you explain more how the transpose is done (I think via the logic in dot to llvm conversion at register level?) and how this is expected to improve global store in the commit message? good for others to understand why this change.

@@ -79,6 +79,8 @@ SmallVector<unsigned> getWarpOrder(Attribute layout);

SmallVector<unsigned> getOrder(Attribute layout);

SmallVector<unsigned> getThreadOrder(Attribute layout);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add some doc to both this one and the above getOrder so that it's easier to know the difference?

mfmaEnc = ttg::AMDMfmaEncodingAttr::get(
oldRetType.getContext(),
/*versionMajor*/ mfmaVersion, /*versionMinor*/ 0, warpsPerTile,
/*instrShape*/ mDim, nDim, isTransposed, CTALayout);
/*instrShape*/ mDim, nDim, /*isTransposed*/ true, CTALayout);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add some comments in the above of why we want to always set it as transposed? It makes it easier to follow for others reading the code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants